Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Introduce more extension points and delete scopes #10758

Merged
merged 13 commits into from
May 16, 2024
Merged

Introduce more extension points and delete scopes #10758

merged 13 commits into from
May 16, 2024

Conversation

kulmann
Copy link
Member

@kulmann kulmann commented Apr 9, 2024 •
edited
Loading

Description

This PR introduces more extension points, allows registering extension points through app bootstrapping and gets rid of the scopes property on the Extension type.

All of the changes are "under the hood", if anything is different in the UI now it's a bug in the PR.

Motivation and Context

Offer extension points throughout our platform so that extension developers can easily hook their extensions into existing extension points.

Types of changes

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to change)
  • Technical debt
  • Tests
  • Documentation
  • Maintenance (e.g. dependency updates or tooling)

@update-docs Update Docs
Copy link

update-docs bot commented Apr 9, 2024

Thanks for opening this pull request! The maintainers of this repository would appreciate it if you would create a changelog item based on your changes.

JammingBen
JammingBen reviewed Apr 11, 2024
View reviewed changes
Copy link
Contributor

@JammingBen JammingBen left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Love to see the scopes go in favour of this 🥳

docs/extension-system/_index.md Outdated Show resolved Hide resolved
dschmidt
dschmidt reviewed Apr 11, 2024
View reviewed changes
docs/extension-system/_index.md Outdated Show resolved Hide resolved
docs/extension-system/_index.md Outdated Show resolved Hide resolved
packages/web-app-files/src/components/FilesList/QuickActions.vue Outdated Show resolved Hide resolved
packages/web-pkg/src/composables/actions/types.ts Outdated Show resolved Hide resolved
packages/web-pkg/src/composables/piniaStores/extensionRegistry/types.ts Outdated Show resolved Hide resolved
packages/web-pkg/src/composables/piniaStores/extensionRegistry/extensionRegistry.ts Show resolved Hide resolved
packages/web-app-files/tests/unit/views/spaces/GenericSpace.spec.ts Outdated Show resolved Hide resolved
packages/web-app-files/src/extensionPoints.ts Outdated Show resolved Hide resolved
packages/web-app-files/src/extensionPoints.ts Show resolved Hide resolved
@kulmann kulmann force-pushed the kill-scopes branch 6 times, most recently from bdb97b6 to dbef7a9 Compare April 24, 2024 04:14
@kulmann kulmann self-assigned this Apr 24, 2024
@kulmann kulmann changed the title Introduce more extension points Introduce more extension points and delete scopes Apr 24, 2024
@kulmann kulmann marked this pull request as ready for review April 24, 2024 05:12
@kulmann kulmann requested review from dschmidt and JammingBen April 24, 2024 05:12
@kulmann
Copy link
Member Author

kulmann commented Apr 24, 2024

@dschmidt @JammingBen ready for a thorough re-review. As far as I can tell the last remaining open question is whether the files app extension points should be defined and registered via the files app or defined in web-pkg and registered via the runtime... I added all extension point ids to the dev docs, so we might as well say that that's the source of truth for existing extension point ids. But might also be a good idea to have the extension point objects available in other apps. What do you think?

@kulmann kulmann force-pushed the kill-scopes branch 3 times, most recently from ae2f48b to 66d9138 Compare May 15, 2024 10:49
dschmidt
dschmidt approved these changes May 15, 2024
View reviewed changes
Copy link
Member

@dschmidt dschmidt left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM 😍

docs/extension-system/_index.md Show resolved Hide resolved
packages/web-app-files/src/extensionPoints.ts Show resolved Hide resolved
packages/web-app-search/src/index.ts Show resolved Hide resolved
packages/web-pkg/src/components/AppBar/AppBar.vue Outdated
@@ -207,6 +210,14 @@ export default defineComponent({
] as FileAction[]
}

const actionExtensions = requestExtensions<ActionExtension>({
id: 'app.files.batch-actions',
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hmm - it's a bit weird to reference an extension point defined in an app in web-pkg ... 👀

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

agreed. as discussed in chat: we'll tackle that in a followup.

packages/web-pkg/src/composables/piniaStores/extensionRegistry/extensionRegistry.ts Outdated Show resolved Hide resolved
packages/web-runtime/src/extensionPoints.ts Outdated Show resolved Hide resolved
packages/web-runtime/src/pages/account.vue Outdated Show resolved Hide resolved
@sonarcloud SonarCloud
Copy link

sonarcloud bot commented May 16, 2024

Quality Gate Passed Quality Gate passed

Issues
1 New issue
0 Accepted issues

Measures
0 Security Hotspots
0.0% Coverage on New Code
0.0% Duplication on New Code

See analysis details on SonarCloud

@kulmann kulmann merged commit 0083f4f into master May 16, 2024
3 checks passed
@delete-merged-branch delete-merged-branch bot deleted the kill-scopes branch May 16, 2024 19:04
ownclouders pushed a commit that referenced this pull request May 16, 2024
@kulmann
Introduce more extension points and delete scopes (#10758)
9f04956 
* feat: introduce extension points
@micbar micbar mentioned this pull request Jun 19, 2024
Closed
24 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@dschmidt dschmidt dschmidt approved these changes

@JammingBen JammingBen Awaiting requested review from JammingBen

Assignees

@kulmann kulmann

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@kulmann @dschmidt @JammingBen